Fix private properties not being allowed in optional chains#60263
Fix private properties not being allowed in optional chains#60263Psychpsyo wants to merge 3 commits intomicrosoft:mainfrom
Conversation
|
@microsoft-github-policy-service agree |
jakebailey
left a comment
There was a problem hiding this comment.
Coming back to this, this PR is missing a few things that would make this work:
- A test that the downlevelling works, so set the test's target to
@target: esnext,es2015. - A new test that verifies some sort of cross-class error happens, e.g. copy and paste the class in the test to
B, and there should be type errors sinceAis the wrong class.
Done that now. |
| if (isOptionalChain && isPrivateIdentifier(propertyAccess.name)) { | ||
| parseErrorAtRange(propertyAccess.name, Diagnostics.An_optional_chain_cannot_contain_private_identifiers); | ||
| } |
There was a problem hiding this comment.
I suspect we need sort of version check to verify that the syntax is available to the syntax we're emitting, like a new grammar check in the checker to verify this.
Unless you're saying that this syntax has always been allowed since feature was standardized?
|
With 6.0 out as the final release vehicle for this codebase, we're closing all PRs that don't fit the merge criteria for post-6.0 patches. If you think this was a mistake and this PR fits the post-6.0 patch criteria, please post to the 6.0 iteration issue with details (specifically, which PR and which patch criteria it satisfies). Next steps for PRs:
|
Fixes #42734
This makes it so that expressions of the form
foo?.#barorfoo?.bar.#bazare no longer seen as invalid, since the relevant JavaScript standards allow these by now and TypeScript claims to be a superset of JavaScript.Specifically, private properties are now allowed inside an optional chain.
This syntax is allowed since tc39/proposal-class-fields#301
The added
this.a = this;inprivateIdentifierChain.1.tsis there so that line 14,this?.a.#b;, does not error becauseamight be undefined, as that's not what this file is testing for.I'm sorry for fixing an issue that is not in the backlog, but this is bothering me personally and has existed for years at this point.
Otherwise, I have verified that:
mainbranchhereby runtestslocally